✨ Implement Boxcutter #1946

thetechnick · 2025-04-30T14:47:07Z

Description

Reviewer Checklist

API Go Documentation
Tests: Unit Tests (and E2E Tests, if appropriate)
Comprehensive Commit Messages
Links to related GitHub Issue(s)

netlify · 2025-04-30T14:47:25Z

❌ Deploy Preview for olmv1 failed.

Name	Link
🔨 Latest commit	`7ba6e02`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/688d222ca34cc20008d33c06

api/v1/clusterextensionrevision_types.go

internal/operator-controller/applier/boxcutter.go

joelanford · 2025-04-30T17:03:46Z

internal/operator-controller/applier/boxcutter.go

+	hasher.Reset()
+
+	printer := spew.ConfigState{
+		Indent:         " ",
+		SortKeys:       true,
+		DisableMethods: true,
+		SpewKeys:       true,
+	}
+	if _, err := printer.Fprintf(hasher, "%#v", objectToWrite); err != nil {
+		panic(err)
+	}


Is this an OLM-ism or a boxcutter-ism? I wonder if json.NewEncoder(hasher).Encode(objectToWrite) would be better?

This is a kubernetes apimachinery thing:
https://github.com/kubernetes/kubernetes/blob/627ce4946ccd6b67bc18f5a4dda92cf6c583e9b7/pkg/util/hash/hash.go#L26-L32
https://github.com/kubernetes/apimachinery/blob/master/pkg/util/dump/dump.go

I'll import from apimachinery next.

internal/operator-controller/applier/boxcutter.go

codecov · 2025-07-15T12:09:46Z

Codecov Report

❌ Patch coverage is 46.99538% with 344 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.85%. Comparing base (8cf5bda) to head (d96b88a).
⚠️ Report is 19 commits behind head on main.

Files with missing lines	Patch %	Lines
api/v1/zz_generated.deepcopy.go	0.00%	114 Missing ⚠️
...controllers/clusterextensionrevision_controller.go	70.44%	59 Missing and 14 partials ⚠️
cmd/operator-controller/main.go	21.34%	63 Missing and 7 partials ⚠️
...perator-controller/rukpak/util/testing/bundlefs.go	0.00%	42 Missing ⚠️
internal/operator-controller/applier/boxcutter.go	73.79%	25 Missing and 13 partials ⚠️
...troller/controllers/clusterextension_controller.go	30.00%	6 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1946      +/-   ##
==========================================
- Coverage   72.61%   69.85%   -2.77%     
==========================================
  Files          78       83       +5     
  Lines        7260     7965     +705     
==========================================
+ Hits         5272     5564     +292     
- Misses       1633     2015     +382     
- Partials      355      386      +31

Flag	Coverage Δ
e2e	`40.09% <3.95%> (-3.46%)`	⬇️
unit	`57.25% <43.60%> (-1.63%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

scripts/install.tpl.sh

tmshort · 2025-07-28T15:57:31Z

config/components/features/boxcutter-runtime/kustomization.yaml

+# DO NOT ADD A NAMESPACE HERE
+---
+apiVersion: kustomize.config.k8s.io/v1alpha1
+kind: Component


This component is not used... it's not being included in the experimental manifests (yet).
See:
https://github.com/operator-framework/operator-controller/blob/main/config/components/base/experimental/kustomization.yaml

That was intentional. Boxcutter runtime is orthogonal to all other feature gates. I think, at least for the time being, it is preferable to e2e tests the other FGs against the standard applier.

cmd/operator-controller/main.go

tmshort · 2025-07-28T19:32:15Z

internal/operator-controller/rukpak/util/testing/bundlefs.go

+	BundlePathCSV         = BundlePathManifests + "/csv.yaml"
+)
+
+func NewBundleFS() fstest.MapFS {


What's the benefit to moving this out of the bundle/source/source_test.go? I'm not seeing it used anywhere else. It just feels as though it's making the PR bigger without benefit?

It's used in the boxcutter applier test [e.g.]

tmshort · 2025-07-28T19:53:18Z

internal/operator-controller/controllers/suite_test.go

-	sch := apimachineryruntime.NewScheme()
-	require.NoError(t, ocv1.AddToScheme(sch))
-	cl, err := client.New(config, client.Options{Scheme: sch})
+	cl, err := client.New(config, client.Options{Scheme: newScheme(t)})


Again, not seeing the benefit of putting this into a function.

used by the controllerextensionrevision_controller test, [e.g.]

tmshort · 2025-07-28T20:40:03Z

go.mod

+replace pkg.package-operator.run/boxcutter => github.com/perdasilva/boxcutter v0.0.0-20250715101157-18ea858f54bd
+


Presumably, temporary, until you get your boxcutter changes merged?

cmd/operator-controller/main.go

openshift-ci · 2025-07-29T08:18:50Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kevinrizza for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…time libs Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

…re component Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

Signed-off-by: Todd Short <tshort@redhat.com>

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 30, 2025

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 30, 2025

joelanford reviewed Apr 30, 2025

View reviewed changes

api/v1/clusterextensionrevision_types.go Show resolved Hide resolved

joelanford reviewed Apr 30, 2025

View reviewed changes

internal/operator-controller/applier/boxcutter.go Outdated Show resolved Hide resolved

joelanford reviewed Apr 30, 2025

View reviewed changes

internal/operator-controller/applier/boxcutter.go Outdated Show resolved Hide resolved

camilamacedo86 mentioned this pull request Jun 28, 2025

✨ Add support for deploying OCI helm charts in OLM v1 #1971

Merged

4 tasks

thetechnick force-pushed the poc-boxcutter branch from 36d9cd7 to b401796 Compare July 7, 2025 13:08

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 7, 2025

perdasilva force-pushed the poc-boxcutter branch 2 times, most recently from 1c23866 to dc66501 Compare July 15, 2025 11:12

perdasilva changed the title ~~POC: Implement Boxcutter~~ ✨ Implement Boxcutter Jul 15, 2025

perdasilva force-pushed the poc-boxcutter branch 6 times, most recently from 35cdf3b to 370bac6 Compare July 21, 2025 16:28

tmshort reviewed Jul 21, 2025

View reviewed changes

scripts/install.tpl.sh Outdated Show resolved Hide resolved

perdasilva force-pushed the poc-boxcutter branch 9 times, most recently from 4f39c38 to 842037e Compare July 23, 2025 15:27

perdasilva force-pushed the poc-boxcutter branch 2 times, most recently from b3f1de0 to 6827d71 Compare July 28, 2025 14:47

tmshort reviewed Jul 28, 2025

View reviewed changes

cmd/operator-controller/main.go Show resolved Hide resolved

tmshort reviewed Jul 28, 2025

View reviewed changes

cmd/operator-controller/main.go Outdated Show resolved Hide resolved

perdasilva force-pushed the poc-boxcutter branch from 6827d71 to 6cd5b57 Compare July 29, 2025 08:18

Per Goncalves da Silva and others added 15 commits August 1, 2025 16:56

Update boxcutter library to branch with latest k8s and controller-run…

8e3683f

…time libs Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

Update go.mod

1b1396c

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

Add ClusterExtensionRevisionAPI

63224c1

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

Add BoxcutterRuntime featuregate

6430597

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

Add Boxcutter applier

ae24b48

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

Add ClusterExtensionRevision controller

9ae89ac

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

Add Boxcutter runtime to main

0b0f5e7

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

Remove ClusterExtensionRevision from crd-docs

71e08b5

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

Update hack/tools/update-crds.sh for ClusterExtensionRevision API

8e0fa98

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

Generate manifests

975f9c2

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

Remove access manager and dynamic cache

2313eef

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

Update boxcutter to v0.3.0, add TrackingCache to Runnables

3a6304a

boxcutter webhook support

b72f9b2

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

add BoxcutterRuntime feature gate to experimental release

2f49013

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

add boxcutter cluster-admin cluster role binding in boxcutter's featu…

294dd8c

…re component Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

thetechnick force-pushed the poc-boxcutter branch from d96b88a to 294dd8c Compare August 1, 2025 14:56

tmshort added 2 commits August 1, 2025 16:04

Boxcutter Preflight

bfeb343

Signed-off-by: Todd Short <tshort@redhat.com>

Boxcutter Preflight mock cleanup

7ba6e02

Signed-off-by: Todd Short <tshort@redhat.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

✨ Implement Boxcutter #1946

✨ Implement Boxcutter #1946

Uh oh!

thetechnick commented Apr 30, 2025

Uh oh!

netlify bot commented Apr 30, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

joelanford Apr 30, 2025

Uh oh!

thetechnick May 12, 2025

Uh oh!

Uh oh!

codecov bot commented Jul 15, 2025 •

edited

Loading

Uh oh!

Uh oh!

tmshort Jul 28, 2025

Uh oh!

perdasilva Jul 29, 2025

Uh oh!

Uh oh!

tmshort Jul 28, 2025

Uh oh!

perdasilva Jul 29, 2025

Uh oh!

tmshort Jul 28, 2025

Uh oh!

perdasilva Jul 29, 2025 •

edited

Loading

Uh oh!

tmshort Jul 28, 2025

Uh oh!

Uh oh!

openshift-ci bot commented Jul 29, 2025

Uh oh!

Uh oh!

		replace pkg.package-operator.run/boxcutter => github.com/perdasilva/boxcutter v0.0.0-20250715101157-18ea858f54bd

✨ Implement Boxcutter #1946

Are you sure you want to change the base?

✨ Implement Boxcutter #1946

Uh oh!

Conversation

thetechnick commented Apr 30, 2025

Description

Reviewer Checklist

Uh oh!

netlify bot commented Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ Deploy Preview for olmv1 failed.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

perdasilva Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

openshift-ci bot commented Jul 29, 2025

Uh oh!

Uh oh!

netlify bot commented Apr 30, 2025 •

edited

Loading

codecov bot commented Jul 15, 2025 •

edited

Loading

perdasilva Jul 29, 2025 •

edited

Loading